Skip to content

Conversation

@Mashimiao
Copy link

config.json is used for creation of container not for
operations of container, we should specify this.

Signed-off-by: Ma Shimiao [email protected]

config.md Outdated
# <a name="containerConfigurationFile" />Container Configuration file

This configuration file contains metadata necessary to implement standard operations against the container.
This configuration file contains metadata necessary to implement standard operations against the creation of a container.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

process.args is used by start and some hooks are used by delete. But kill doesn't need anything from then config. So we can draw a distinction here, but your current wording isn't quite it.

I'd rather link "standard operations" to the runtime.md section and maybe weaken it to "some standard operations". But I'd avoid calling out creation without also mentioning the other impacted operations.

@tianon
Copy link
Member

tianon commented May 11, 2017

I think I tend to agree with @wking here -- in my mind, the config.json (and I suppose the rest of the bundle) is the specification of the desired state of the container, and set of information for how to find/reference/interact with it.

Regardless, I think this is something we could discuss further clarifying post-1.0?

@Mashimiao Mashimiao force-pushed the config-specify-config branch from 92b2110 to b2ec523 Compare May 11, 2017 10:43
@Mashimiao Mashimiao changed the title config.md: specify config used for creation of container config.md: specify config usage May 11, 2017
@Mashimiao
Copy link
Author

@tianon @wking updated as @wking suggested

@tianon
Copy link
Member

tianon commented May 11, 2017

LGTM

Approved with PullApprove

@mrunalp
Copy link
Contributor

mrunalp commented May 12, 2017

Without the config.json there is no container and no operations to run against a container so I am in favor of closing this PR.

config.md Outdated
# <a name="containerConfigurationFile" />Container Configuration file

This configuration file contains metadata necessary to implement standard operations against the container.
This configuration file contains metadata necessary to implement some [standard operations](runtime.md#operations) against the container.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets remove the "some" part but keep the link

config.json is used some operations of container,
we should specify this.

Signed-off-by: Ma Shimiao <[email protected]>
@Mashimiao Mashimiao force-pushed the config-specify-config branch from b2ec523 to 773ebb2 Compare May 13, 2017 00:38
@Mashimiao
Copy link
Author

OK, updated.

Copy link
Contributor

@mrunalp mrunalp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mrunalp
Copy link
Contributor

mrunalp commented May 13, 2017

LGTM

Approved with PullApprove

1 similar comment
@tianon
Copy link
Member

tianon commented May 13, 2017

LGTM

Approved with PullApprove

@tianon tianon merged commit 559acdf into opencontainers:master May 13, 2017
@vbatts vbatts mentioned this pull request Jul 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants